Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable mandatory TLS verification with --insecure #1401

Closed

Conversation

d4xfe
Copy link
Contributor

@d4xfe d4xfe commented Apr 29, 2024

getpeercert() only returns the certificate with enabled certificate validation. Because of this, I updated it to getpeercert(True), which will return a binary blob of the servers certificate regardless of the verification mode.

I added the util method cert_der_to_dict using ssl internals to convert the blob to a python dict like getpeercert() would.
The flag --insecure was introduced to disable certificate validation of ssl sockets.
This allows TLS interception when the server is using a self-signed certificate.

This is my first pull request on GitHub and I'm still a bit confused. So if I should change anything let me know.

@abhinavsingh
Copy link
Owner

@d4xfe Thank you, looks excellent considering this is your first PR. Congratulations.

Can you please:

  1. Resolve conflicts
  2. Sync your branch with develop. For some reasons, I see diffs in your PR which are not part of your changes

@abhinavsingh abhinavsingh self-requested a review April 30, 2024 03:00
@abhinavsingh abhinavsingh added the bot:chronographer:skip PR using this label is exempted from CHANGELOG management label Apr 30, 2024
@d4xfe
Copy link
Contributor Author

d4xfe commented Apr 30, 2024

@abhinavsingh Thanks. I've synced the branches and I don't conflicts anymore.

Copy link

codecov bot commented Aug 10, 2024

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.37%. Comparing base (39854e1) to head (ec87033).
Report is 21 commits behind head on develop.

Files with missing lines Patch % Lines
proxy/common/utils.py 81.81% 1 Missing and 1 partial ⚠️
proxy/core/connection/server.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1401      +/-   ##
===========================================
- Coverage    84.57%   84.37%   -0.20%     
===========================================
  Files          177      178       +1     
  Lines         8103     8130      +27     
  Branches      1239     1242       +3     
===========================================
+ Hits          6853     6860       +7     
- Misses        1052     1059       +7     
- Partials       198      211      +13     
Flag Coverage Δ
GHA 84.36% <87.09%> (-0.05%) ⬇️
Linux 83.99% <87.09%> (-0.04%) ⬇️
Python 3.10 85.13% <82.60%> (-0.02%) ⬇️
Python 3.11 84.31% <87.09%> (+<0.01%) ⬆️
Python 3.12 79.36% <87.09%> (-4.96%) ⬇️
Python 3.6 ?
Python 3.7 ?
Python 3.8 ?
Python 3.9 85.21% <82.60%> (-0.02%) ⬇️
Windows ?
macOS 84.36% <87.09%> (-0.05%) ⬇️
pytest 84.36% <87.09%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abhinavsingh
Copy link
Owner

@d4xfe Thank you for this work and the PR. There are some workflow issues. I am going create a new PR based on top of your contribution and try to this ship.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:skip PR using this label is exempted from CHANGELOG management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants